From: Pierre Chifflier Date: Sun, 30 Mar 2025 10:03:02 +0000 (+0200) Subject: CVE-2024-32663-1 X-Git-Tag: archive/raspbian/1%6.0.1-3+rpi1+deb11u1^2~6 X-Git-Url: https://dgit.raspbian.org/%22http://www.example.com/cgi/%22/%22http:/www.example.com/cgi/%22?a=commitdiff_plain;h=6fbc7721ca4ef80bc864df6543f80bda912cf33a;p=suricata.git CVE-2024-32663-1 commit 08d93f7c3762781b743f88f9fdc4389eb9c3eb64 Author: Philippe Antoine Date: Wed Mar 27 14:33:54 2024 +0100 http2: use a reference counter for headers Ticket: 6892 As HTTP hpack header compression allows one single byte to express a previously seen arbitrary-size header block (name+value) we should avoid to copy the vectors data, but just point to the same data, while reamining memory safe, even in the case of later headers eviction from the dybnamic table. Rust std solution is Rc, and the use of clone, so long as the data is accessed by only one thread. (cherry picked from commit 390f09692eb99809c679d3f350c7cc185d163e1a) Gbp-Pq: Name CVE-2024-32663-1.patch --- diff --git a/rust/src/http2/detect.rs b/rust/src/http2/detect.rs index 59180263..540d4227 100644 --- a/rust/src/http2/detect.rs +++ b/rust/src/http2/detect.rs @@ -23,6 +23,7 @@ use crate::core::STREAM_TOSERVER; use std::ffi::CStr; use std::mem::transmute; use std::str::FromStr; +use std::rc::Rc; fn http2_tx_has_frametype( tx: &mut HTTP2Transaction, direction: u8, value: u8, @@ -557,8 +558,8 @@ fn http2_tx_set_header(state: &mut HTTP2State, name: &[u8], input: &[u8]) { }; let mut blocks = Vec::new(); let b = parser::HTTP2FrameHeaderBlock { - name: name.to_vec(), - value: input.to_vec(), + name: Rc::new(name.to_vec()), + value: Rc::new(input.to_vec()), error: parser::HTTP2HeaderDecodeStatus::HTTP2HeaderDecodeSuccess, sizeupdate: 0, }; diff --git a/rust/src/http2/parser.rs b/rust/src/http2/parser.rs index 61e4557f..036ebd2f 100644 --- a/rust/src/http2/parser.rs +++ b/rust/src/http2/parser.rs @@ -25,6 +25,7 @@ use nom::Err; use nom::IResult; use std::fmt; use std::str::FromStr; +use std::rc::Rc; #[repr(u8)] #[derive(Clone, Copy, PartialEq, FromPrimitive, Debug)] @@ -281,8 +282,8 @@ fn http2_frame_header_static(n: u64, dyn_headers: &HTTP2DynTable) -> Option 0 { return Some(HTTP2FrameHeaderBlock { - name: name.as_bytes().to_vec(), - value: value.as_bytes().to_vec(), + name: Rc::new(name.as_bytes().to_vec()), + value: Rc::new(value.as_bytes().to_vec()), error: HTTP2HeaderDecodeStatus::HTTP2HeaderDecodeSuccess, sizeupdate: 0, }); @@ -290,23 +291,23 @@ fn http2_frame_header_static(n: u64, dyn_headers: &HTTP2DynTable) -> Option, - pub value: Vec, + // Use Rc reference counted so that indexed headers do not get copied. + // Otherwise, this leads to quadratic complexity in memory occupation. + pub name: Rc>, + pub value: Rc>, pub error: HTTP2HeaderDecodeStatus, pub sizeupdate: u64, } @@ -386,7 +389,7 @@ fn http2_parse_headers_block_literal_common<'a>( ) -> IResult<&'a [u8], HTTP2FrameHeaderBlock> { let (i3, name, error) = if index == 0 { match http2_parse_headers_block_string(input) { - Ok((r, n)) => Ok((r, n, HTTP2HeaderDecodeStatus::HTTP2HeaderDecodeSuccess)), + Ok((r, n)) => Ok((r, Rc::new(n), HTTP2HeaderDecodeStatus::HTTP2HeaderDecodeSuccess)), Err(e) => Err(e), } } else { @@ -398,7 +401,7 @@ fn http2_parse_headers_block_literal_common<'a>( )), None => Ok(( input, - Vec::new(), + Rc::new(Vec::new()), HTTP2HeaderDecodeStatus::HTTP2HeaderDecodeNotIndexed, )), } @@ -408,7 +411,7 @@ fn http2_parse_headers_block_literal_common<'a>( i4, HTTP2FrameHeaderBlock { name, - value, + value: Rc::new(value), error, sizeupdate: 0, }, @@ -436,8 +439,8 @@ fn http2_parse_headers_block_literal_incindex<'a>( match r { Ok((r, head)) => { let headcopy = HTTP2FrameHeaderBlock { - name: head.name.to_vec(), - value: head.value.to_vec(), + name: head.name.clone(), + value: head.value.clone(), error: head.error, sizeupdate: 0, }; @@ -554,8 +557,8 @@ fn http2_parse_headers_block_dynamic_size<'a>( return Ok(( i3, HTTP2FrameHeaderBlock { - name: Vec::new(), - value: Vec::new(), + name: Vec::new().into(), + value: Vec::new().into(), error: HTTP2HeaderDecodeStatus::HTTP2HeaderDecodeIntegerOverflow, sizeupdate: 0, }, @@ -573,8 +576,8 @@ fn http2_parse_headers_block_dynamic_size<'a>( return Ok(( i3, HTTP2FrameHeaderBlock { - name: Vec::new(), - value: Vec::new(), + name: Rc::new(Vec::new()), + value: Rc::new(Vec::new()), error: HTTP2HeaderDecodeStatus::HTTP2HeaderDecodeSizeUpdate, sizeupdate: maxsize2, }, @@ -928,8 +931,8 @@ mod tests { match r0 { Ok((remainder, hd)) => { // Check the first message. - assert_eq!(hd.name, ":method".as_bytes().to_vec()); - assert_eq!(hd.value, "GET".as_bytes().to_vec()); + assert_eq!(hd.name, ":method".as_bytes().to_vec().into()); + assert_eq!(hd.value, "GET".as_bytes().to_vec().into()); // And we should have no bytes left. assert_eq!(remainder.len(), 0); } @@ -945,8 +948,8 @@ mod tests { match r1 { Ok((remainder, hd)) => { // Check the first message. - assert_eq!(hd.name, "accept".as_bytes().to_vec()); - assert_eq!(hd.value, "*/*".as_bytes().to_vec()); + assert_eq!(hd.name, "accept".as_bytes().to_vec().into()); + assert_eq!(hd.value, "*/*".as_bytes().to_vec().into()); // And we should have no bytes left. assert_eq!(remainder.len(), 0); assert_eq!(dynh.table.len(), 1); @@ -965,8 +968,8 @@ mod tests { match result { Ok((remainder, hd)) => { // Check the first message. - assert_eq!(hd.name, ":authority".as_bytes().to_vec()); - assert_eq!(hd.value, "localhost:3000".as_bytes().to_vec()); + assert_eq!(hd.name, ":authority".as_bytes().to_vec().into()); + assert_eq!(hd.value, "localhost:3000".as_bytes().to_vec().into()); // And we should have no bytes left. assert_eq!(remainder.len(), 0); assert_eq!(dynh.table.len(), 2); @@ -983,8 +986,8 @@ mod tests { match r3 { Ok((remainder, hd)) => { // same as before - assert_eq!(hd.name, ":authority".as_bytes().to_vec()); - assert_eq!(hd.value, "localhost:3000".as_bytes().to_vec()); + assert_eq!(hd.name, ":authority".as_bytes().to_vec().into()); + assert_eq!(hd.value, "localhost:3000".as_bytes().to_vec().into()); // And we should have no bytes left. assert_eq!(remainder.len(), 0); assert_eq!(dynh.table.len(), 2); @@ -1019,8 +1022,8 @@ mod tests { match r2 { Ok((remainder, hd)) => { // Check the first message. - assert_eq!(hd.name, ":path".as_bytes().to_vec()); - assert_eq!(hd.value, "/doc/manual/html/index.html".as_bytes().to_vec()); + assert_eq!(hd.name, ":path".as_bytes().to_vec().into()); + assert_eq!(hd.value, "/doc/manual/html/index.html".as_bytes().to_vec().into()); // And we should have no bytes left. assert_eq!(remainder.len(), 0); assert_eq!(dynh.table.len(), 2);